Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔄 Support recursive includes in myst and tex #1082

Merged
merged 8 commits into from
Apr 10, 2024
Merged

Conversation

fwkoch
Copy link
Collaborator

@fwkoch fwkoch commented Apr 10, 2024

This adds more robust support for "recursive includes" - these are where one file includes another file which includes a third file, etc.

Additionally:

  • Circular include dependencies error nicely (including printing the "include-stack") - no infinite loops:
    image
  • tex source files with \input macros resolve and reload recursively as well

Resolves:

@@ -116,13 +113,6 @@ export async function loadFile(
const { sha256, useCache } = checkCache(cache, content, file);
if (useCache) break;
const tex = new TexParser(content, vfile);
await includeDirectiveTransform(tex.ast as GenericParent, vfile, {
Copy link
Collaborator Author

@fwkoch fwkoch Apr 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Live reload was not working well for tex include content. The fastProcessFile function was getting triggered correctly for dependent files, but this transform was never run on the file with \input{...} (useCache was still true).

Now, when this function is called in processMdast it handles tex as well, so live reload behaviour matches md processing, regardless of useCache for file loading. (And calling the function here is unnecessary.)

*/
export const makeFileLoader = (session: ISession, baseFile: string) => (fullFile: string) => {
session.store.dispatch(
watch.actions.addLocalDependency({
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These include dependencies do not show up in the page json "dependency" field, but they are respected for live site reloading.

I think that's ok - the json "dependencies" are used for loading data from other pages; in the case of include, this data is actually moved to the page itself.

🤔 Maybe if we update include to allow including partial snippets from md files we may need to load the rest of the data from that page...? (Similar to embed pulling in a single figure.) But for now, it's unnecessary.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, the included files are a local convinience, not for how the final documents work.

@rowanc1
Copy link
Collaborator

rowanc1 commented Apr 10, 2024

Works really well locally in both latex and markdown - and a mix as well, which is kinda cool:

image

@rowanc1
Copy link
Collaborator

rowanc1 commented Apr 10, 2024

Added some docs, this is good to go in my opinion!

image

Copy link
Collaborator

@rowanc1 rowanc1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, fun to see the implementation. Nice work @fwkoch.

@fwkoch fwkoch merged commit d17f680 into main Apr 10, 2024
4 checks passed
@fwkoch fwkoch deleted the feat/recursive-include branch April 10, 2024 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants